-
Notifications
You must be signed in to change notification settings - Fork 133
fix: make usage chunk in stream mode of gemini compatible with openai #1503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
3fc0bf1 to
35bff4c
Compare
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (61.90%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1503 +/- ##
==========================================
- Coverage 83.26% 83.24% -0.02%
==========================================
Files 137 137
Lines 12059 12069 +10
==========================================
+ Hits 10041 10047 +6
- Misses 1411 1413 +2
- Partials 607 609 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/retest |
|
@yuzisun can you take a look at this one? |
Signed-off-by: yxia216 <[email protected]>
… a tool call (envoyproxy#1486) **Description** Finish reason should be tool calls if the model returns a tool call response. In vertex api, there is no tool call finish reason, thus need a work around to make it compatible. --------- Signed-off-by: yxia216 <[email protected]> Co-authored-by: Dan Sun <[email protected]> Signed-off-by: yxia216 <[email protected]>
…oxy#1491) **Description** This decouples backendauth & headermutator packages from extproc specifics. As we are looking to migrate to dynamic modules, this is a necessary refactoring work to make the code as reusable as possible. **Related Issues/PRs (if applicable)** Preliminary for envoyproxy#90 --------- Signed-off-by: Takeshi Yoneda <[email protected]> Signed-off-by: yxia216 <[email protected]>
Signed-off-by: yxia216 <[email protected]>
Signed-off-by: yxia216 <[email protected]>
Signed-off-by: yxia216 <[email protected]>
66eaa8c to
30a3391
Compare
Signed-off-by: yxia216 <[email protected]>
|
/retest |
1 similar comment
|
/retest |
|
/retest |
1 similar comment
|
/retest |
| } | ||
|
|
||
| if span != nil { | ||
| span.RecordResponseChunk(openAIChunk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like copy/paste error, should be usageChunk ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated! Thanks a lot for the comment!
Signed-off-by: yxia216 <[email protected]>
Description
Users found the simply use "usage" information does not work for streaming responses of gemini models.

This is because for openai models, the usage chunk would be a separate chunk. For example, this is an example response from gpt-4o:
There is a finish_reason chunk, and then a usage chunk.
Thus, want to make it compatible with Openai. (Actually, in anthropic translation, it's already compatible)